fix: rename CLOCK_SKEW and separate client/server user case (#863) * fix: rename CLOCK_SKEW and separate client/server user case * update clock skew to 20s
diff --git a/google/auth/_helpers.py b/google/auth/_helpers.py index 11c6b1a..55adf5b 100644 --- a/google/auth/_helpers.py +++ b/google/auth/_helpers.py
@@ -20,8 +20,11 @@ import urllib -CLOCK_SKEW_SECS = 60 # 60 seconds -CLOCK_SKEW = datetime.timedelta(seconds=CLOCK_SKEW_SECS) +# Token server doesn't provide a new a token when doing refresh unless the +# token is expiring within 30 seconds, so refresh threshold should not be +# more than 30 seconds. Otherwise auth lib will send tons of refresh requests +# until 30 seconds before the expiration, and cause a spike of CPU usage. +REFRESH_THRESHOLD = datetime.timedelta(seconds=20) def copy_docstring(source_class):
diff --git a/google/auth/credentials.py b/google/auth/credentials.py index 6356f54..8d9974c 100644 --- a/google/auth/credentials.py +++ b/google/auth/credentials.py
@@ -62,7 +62,7 @@ # Remove 10 seconds from expiry to err on the side of reporting # expiration early so that we avoid the 401-refresh-retry loop. - skewed_expiry = self.expiry - _helpers.CLOCK_SKEW + skewed_expiry = self.expiry - _helpers.REFRESH_THRESHOLD return _helpers.utcnow() >= skewed_expiry @property
diff --git a/google/auth/jwt.py b/google/auth/jwt.py index 1bc7e5e..bb9ffae 100644 --- a/google/auth/jwt.py +++ b/google/auth/jwt.py
@@ -167,12 +167,14 @@ return header -def _verify_iat_and_exp(payload): +def _verify_iat_and_exp(payload, clock_skew_in_seconds=0): """Verifies the ``iat`` (Issued At) and ``exp`` (Expires) claims in a token payload. Args: payload (Mapping[str, str]): The JWT payload. + clock_skew_in_seconds (int): The clock skew used for `iat` and `exp` + validation. Raises: ValueError: if any checks failed. @@ -188,7 +190,7 @@ iat = payload["iat"] # Err on the side of accepting a token that is slightly early to account # for clock skew. - earliest = iat - _helpers.CLOCK_SKEW_SECS + earliest = iat - clock_skew_in_seconds if now < earliest: raise ValueError( "Token used too early, {} < {}. Check that your computer's clock is set correctly.".format( @@ -200,12 +202,12 @@ exp = payload["exp"] # Err on the side of accepting a token that is slightly out of date # to account for clow skew. - latest = exp + _helpers.CLOCK_SKEW_SECS + latest = exp + clock_skew_in_seconds if latest < now: raise ValueError("Token expired, {} < {}".format(latest, now)) -def decode(token, certs=None, verify=True, audience=None): +def decode(token, certs=None, verify=True, audience=None, clock_skew_in_seconds=0): """Decode and verify a JWT. Args: @@ -221,6 +223,8 @@ audience (str or list): The audience claim, 'aud', that this JWT should contain. Or a list of audience claims. If None then the JWT's 'aud' parameter is not verified. + clock_skew_in_seconds (int): The clock skew used for `iat` and `exp` + validation. Returns: Mapping[str, str]: The deserialized JSON payload in the JWT. @@ -271,7 +275,7 @@ raise ValueError("Could not verify token signature.") # Verify the issued at and created times in the payload. - _verify_iat_and_exp(payload) + _verify_iat_and_exp(payload, clock_skew_in_seconds) # Check audience. if audience is not None: diff --git a/google/oauth2/credentials.py b/google/oauth2/credentials.py index e259f78..6d34edf 100644 --- a/google/oauth2/credentials.py +++ b/google/oauth2/credentials.py
@@ -270,7 +270,7 @@ raise exceptions.RefreshError( "The refresh_handler returned expiry is not a datetime object." ) - if _helpers.utcnow() >= expiry - _helpers.CLOCK_SKEW: + if _helpers.utcnow() >= expiry - _helpers.REFRESH_THRESHOLD: raise exceptions.RefreshError( "The credentials returned by the refresh_handler are " "already expired." @@ -359,7 +359,7 @@ expiry.rstrip("Z").split(".")[0], "%Y-%m-%dT%H:%M:%S" ) else: - expiry = _helpers.utcnow() - _helpers.CLOCK_SKEW + expiry = _helpers.utcnow() - _helpers.REFRESH_THRESHOLD # process scopes, which needs to be a seq if scopes is None and "scopes" in info: diff --git a/tests/compute_engine/test_credentials.py b/tests/compute_engine/test_credentials.py index ebe9aa5..81cc6db 100644 --- a/tests/compute_engine/test_credentials.py +++ b/tests/compute_engine/test_credentials.py
@@ -64,7 +64,7 @@ @mock.patch( "google.auth._helpers.utcnow", - return_value=datetime.datetime.min + _helpers.CLOCK_SKEW, + return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD, ) @mock.patch("google.auth.compute_engine._metadata.get", autospec=True) def test_refresh_success(self, get, utcnow): @@ -98,7 +98,7 @@ @mock.patch( "google.auth._helpers.utcnow", - return_value=datetime.datetime.min + _helpers.CLOCK_SKEW, + return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD, ) @mock.patch("google.auth.compute_engine._metadata.get", autospec=True) def test_refresh_success_with_scopes(self, get, utcnow): diff --git a/tests/oauth2/test_credentials.py b/tests/oauth2/test_credentials.py index b6a80e3..243f97d 100644 --- a/tests/oauth2/test_credentials.py +++ b/tests/oauth2/test_credentials.py
@@ -115,7 +115,7 @@ @mock.patch("google.oauth2.reauth.refresh_grant", autospec=True) @mock.patch( "google.auth._helpers.utcnow", - return_value=datetime.datetime.min + _helpers.CLOCK_SKEW, + return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD, ) def test_refresh_success(self, unused_utcnow, refresh_grant): token = "token" @@ -175,7 +175,7 @@ @mock.patch("google.oauth2.reauth.refresh_grant", autospec=True) @mock.patch( "google.auth._helpers.utcnow", - return_value=datetime.datetime.min + _helpers.CLOCK_SKEW, + return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD, ) def test_refresh_with_refresh_token_and_refresh_handler( self, unused_utcnow, refresh_grant @@ -361,7 +361,7 @@ @mock.patch("google.auth._helpers.utcnow", return_value=datetime.datetime.min) def test_refresh_with_refresh_handler_expired_token(self, unused_utcnow): - expected_expiry = datetime.datetime.min + _helpers.CLOCK_SKEW + expected_expiry = datetime.datetime.min + _helpers.REFRESH_THRESHOLD # Simulate refresh handler returns an expired token. refresh_handler = mock.Mock(return_value=("TOKEN", expected_expiry)) scopes = ["email", "profile"] @@ -391,7 +391,7 @@ @mock.patch("google.oauth2.reauth.refresh_grant", autospec=True) @mock.patch( "google.auth._helpers.utcnow", - return_value=datetime.datetime.min + _helpers.CLOCK_SKEW, + return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD, ) def test_credentials_with_scopes_requested_refresh_success( self, unused_utcnow, refresh_grant @@ -457,7 +457,7 @@ @mock.patch("google.oauth2.reauth.refresh_grant", autospec=True) @mock.patch( "google.auth._helpers.utcnow", - return_value=datetime.datetime.min + _helpers.CLOCK_SKEW, + return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD, ) def test_credentials_with_only_default_scopes_requested( self, unused_utcnow, refresh_grant @@ -521,7 +521,7 @@ @mock.patch("google.oauth2.reauth.refresh_grant", autospec=True) @mock.patch( "google.auth._helpers.utcnow", - return_value=datetime.datetime.min + _helpers.CLOCK_SKEW, + return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD, ) def test_credentials_with_scopes_returned_refresh_success( self, unused_utcnow, refresh_grant @@ -588,7 +588,7 @@ @mock.patch("google.oauth2.reauth.refresh_grant", autospec=True) @mock.patch( "google.auth._helpers.utcnow", - return_value=datetime.datetime.min + _helpers.CLOCK_SKEW, + return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD, ) def test_credentials_with_scopes_refresh_failure_raises_refresh_error( self, unused_utcnow, refresh_grant diff --git a/tests/test_credentials.py b/tests/test_credentials.py index 0633b38..2de6388 100644 --- a/tests/test_credentials.py +++ b/tests/test_credentials.py
@@ -46,7 +46,9 @@ # Set the expiration to one second more than now plus the clock skew # accomodation. These credentials should be valid. credentials.expiry = ( - datetime.datetime.utcnow() + _helpers.CLOCK_SKEW + datetime.timedelta(seconds=1) + datetime.datetime.utcnow() + + _helpers.REFRESH_THRESHOLD + + datetime.timedelta(seconds=1) ) assert credentials.valid
diff --git a/tests/test_downscoped.py b/tests/test_downscoped.py index 795ec29..9ca95f5 100644 --- a/tests/test_downscoped.py +++ b/tests/test_downscoped.py
@@ -669,7 +669,9 @@ # Set the expiration to one second more than now plus the clock skew # accommodation. These credentials should be valid. credentials.expiry = ( - datetime.datetime.min + _helpers.CLOCK_SKEW + datetime.timedelta(seconds=1) + datetime.datetime.min + + _helpers.REFRESH_THRESHOLD + + datetime.timedelta(seconds=1) ) assert credentials.valid
diff --git a/tests/test_external_account.py b/tests/test_external_account.py index e8297da..df6174f 100644 --- a/tests/test_external_account.py +++ b/tests/test_external_account.py
@@ -976,7 +976,9 @@ # Set the expiration to one second more than now plus the clock skew # accomodation. These credentials should be valid. credentials.expiry = ( - datetime.datetime.min + _helpers.CLOCK_SKEW + datetime.timedelta(seconds=1) + datetime.datetime.min + + _helpers.REFRESH_THRESHOLD + + datetime.timedelta(seconds=1) ) assert credentials.valid @@ -1027,7 +1029,9 @@ # Set the expiration to one second more than now plus the clock skew # accomodation. These credentials should be valid. credentials.expiry = ( - datetime.datetime.min + _helpers.CLOCK_SKEW + datetime.timedelta(seconds=1) + datetime.datetime.min + + _helpers.REFRESH_THRESHOLD + + datetime.timedelta(seconds=1) ) assert credentials.valid
diff --git a/tests/test_iam.py b/tests/test_iam.py index 30ce227..e9eca58 100644 --- a/tests/test_iam.py +++ b/tests/test_iam.py
@@ -45,7 +45,7 @@ super(CredentialsImpl, self).__init__() self.token = "token" # Force refresh - self.expiry = datetime.datetime.min + _helpers.CLOCK_SKEW + self.expiry = datetime.datetime.min + _helpers.REFRESH_THRESHOLD def refresh(self, request): pass
diff --git a/tests/test_impersonated_credentials.py b/tests/test_impersonated_credentials.py index 126c4c3..3dbb6ca 100644 --- a/tests/test_impersonated_credentials.py +++ b/tests/test_impersonated_credentials.py
@@ -211,11 +211,11 @@ credentials = self.make_credentials(lifetime=None) # Source credentials is refreshed only if it is expired within - # _helpers.CLOCK_SKEW from now. We add a time_skew to the expiry, so + # _helpers.REFRESH_THRESHOLD from now. We add a time_skew to the expiry, so # source credentials is refreshed only if time_skew <= 0. credentials._source_credentials.expiry = ( _helpers.utcnow() - + _helpers.CLOCK_SKEW + + _helpers.REFRESH_THRESHOLD + datetime.timedelta(seconds=time_skew) ) credentials._source_credentials.token = "Token" @@ -238,7 +238,7 @@ assert not credentials.expired # Source credentials is refreshed only if it is expired within - # _helpers.CLOCK_SKEW + # _helpers.REFRESH_THRESHOLD if time_skew > 0: source_cred_refresh.assert_not_called() else:
diff --git a/tests/test_jwt.py b/tests/test_jwt.py index 0dd7fa9..ba7277c 100644 --- a/tests/test_jwt.py +++ b/tests/test_jwt.py
@@ -197,7 +197,7 @@ } ) with pytest.raises(ValueError) as excinfo: - jwt.decode(token, PUBLIC_CERT_BYTES) + jwt.decode(token, PUBLIC_CERT_BYTES, clock_skew_in_seconds=59) assert excinfo.match(r"Token used too early") @@ -210,10 +210,40 @@ } ) with pytest.raises(ValueError) as excinfo: - jwt.decode(token, PUBLIC_CERT_BYTES) + jwt.decode(token, PUBLIC_CERT_BYTES, clock_skew_in_seconds=59) assert excinfo.match(r"Token expired") +def test_decode_success_with_no_clock_skew(token_factory): + token = token_factory( + claims={ + "exp": _helpers.datetime_to_secs( + _helpers.utcnow() + datetime.timedelta(seconds=1) + ), + "iat": _helpers.datetime_to_secs( + _helpers.utcnow() - datetime.timedelta(seconds=1) + ), + } + ) + + jwt.decode(token, PUBLIC_CERT_BYTES) + + +def test_decode_success_with_custom_clock_skew(token_factory): + token = token_factory( + claims={ + "exp": _helpers.datetime_to_secs( + _helpers.utcnow() + datetime.timedelta(seconds=2) + ), + "iat": _helpers.datetime_to_secs( + _helpers.utcnow() - datetime.timedelta(seconds=2) + ), + } + ) + + jwt.decode(token, PUBLIC_CERT_BYTES, clock_skew_in_seconds=1) + + def test_decode_bad_token_wrong_audience(token_factory): token = token_factory() audience = "audience2@example.com" diff --git a/tests/transport/test_grpc.py b/tests/transport/test_grpc.py index 926c1bc..3437658 100644 --- a/tests/transport/test_grpc.py +++ b/tests/transport/test_grpc.py
@@ -80,7 +80,7 @@ def test_call_refresh(self): credentials = CredentialsStub() - credentials.expiry = datetime.datetime.min + _helpers.CLOCK_SKEW + credentials.expiry = datetime.datetime.min + _helpers.REFRESH_THRESHOLD request = mock.create_autospec(transport.Request) plugin = google.auth.transport.grpc.AuthMetadataPlugin(credentials, request)
diff --git a/tests_async/oauth2/test_credentials_async.py b/tests_async/oauth2/test_credentials_async.py index bc89392..06c9141 100644 --- a/tests_async/oauth2/test_credentials_async.py +++ b/tests_async/oauth2/test_credentials_async.py
@@ -62,7 +62,7 @@ @mock.patch("google.oauth2._reauth_async.refresh_grant", autospec=True) @mock.patch( "google.auth._helpers.utcnow", - return_value=datetime.datetime.min + _helpers.CLOCK_SKEW, + return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD, ) @pytest.mark.asyncio async def test_refresh_success(self, unused_utcnow, refresh_grant): @@ -124,7 +124,7 @@ @mock.patch("google.oauth2._reauth_async.refresh_grant", autospec=True) @mock.patch( "google.auth._helpers.utcnow", - return_value=datetime.datetime.min + _helpers.CLOCK_SKEW, + return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD, ) @pytest.mark.asyncio async def test_credentials_with_scopes_requested_refresh_success( @@ -188,7 +188,7 @@ @mock.patch("google.oauth2._reauth_async.refresh_grant", autospec=True) @mock.patch( "google.auth._helpers.utcnow", - return_value=datetime.datetime.min + _helpers.CLOCK_SKEW, + return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD, ) @pytest.mark.asyncio async def test_credentials_with_scopes_returned_refresh_success( @@ -251,7 +251,7 @@ @mock.patch("google.oauth2._reauth_async.refresh_grant", autospec=True) @mock.patch( "google.auth._helpers.utcnow", - return_value=datetime.datetime.min + _helpers.CLOCK_SKEW, + return_value=datetime.datetime.min + _helpers.REFRESH_THRESHOLD, ) @pytest.mark.asyncio async def test_credentials_with_scopes_refresh_failure_raises_refresh_error( diff --git a/tests_async/test_credentials_async.py b/tests_async/test_credentials_async.py index 0a48908..5315483 100644 --- a/tests_async/test_credentials_async.py +++ b/tests_async/test_credentials_async.py
@@ -46,7 +46,9 @@ # Set the expiration to one second more than now plus the clock skew # accomodation. These credentials should be valid. credentials.expiry = ( - datetime.datetime.utcnow() + _helpers.CLOCK_SKEW + datetime.timedelta(seconds=1) + datetime.datetime.utcnow() + + _helpers.REFRESH_THRESHOLD + + datetime.timedelta(seconds=1) ) assert credentials.valid
